Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.64%. Comparing base (b9b1c55) to head (985258d).
Report is 3 commits behind head on main.

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   98.60%   98.64%   +0.03%     
==========================================
  Files          31       32       +1     
  Lines        1793     1841      +48     
  Branches      220      229       +9     
==========================================
+ Hits         1768     1816      +48     
  Misses         24       24              
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

• should maybe be consistent about capitalising the column headers - currently all the drug names are capitalised but cluster and filename are not,
• I think silently putting "run for cluster assignment" into the file seems a bit strange. I think it would be better to warn user "Cluster values will be blank in exported file, as analysis has not yet run. Export anyway?" - and get them to confirm, then just leave cluster values empty.
• Commas aren't escaped in the csv generation, so you end up with an extra column in the row if you've got a filename with a comma in it (should wrap these values in double quotes I think). I tried this and found that that sample failed analysis anyway - probably because of a similar issue in the back end! But that showed up another issue with putting "run analysis.." if the cluster is missing, as it shows that for failed samples too, where running won't make a difference. I think having cluster be blank for those samples would be fine too.

1. agreed done
2. I can't entirely agree with this as users that use beebop know whats going on. so if they export without cluster assignment that should be fine... putting a dialog may be annoying and seems like its a bad thing they are doing (when its entirely valid).
3. Good catches !!!! have done.. and just made cluster column blank if not present

I can't entirely agree with this as users that use beebop know whats going on. so if they export without cluster assignment that should be fine... putting a dialog may be annoying and seems like its a bad thing they are doing (when its entirely valid).

I see what you mean, but it just seems weird to pollute output data with a user instruction, especially if we're assuming that they know what they're doing... Could include 'don't warn again' in the dialog, or could pop up some non-intrusive toast saying "Exported without cluster values (run analysis to include next time)" or something like that.

and just made cluster column blank if not present

oh, ok!

users that use beebop know whats going on

Dangerous assumption! 😆

I see what you mean, but it just seems weird to pollute output data with a user instruction, especially if we're assuming that they know what they're doing... Could include 'don't warn again' in the dialog, or could pop up some non-intrusive toast saying "Exported without cluster values (run analysis to include next time)" or something like that.

yup agree there shouldn't be a weird user instruction in output, thus have made it empty if not present